Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use Arachnid CREATE2 Deployer For Some Contracts #482

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Use Arachnid CREATE2 Deployer For Some Contracts #482

merged 1 commit into from
Aug 15, 2024

Conversation

nlordell
Copy link
Collaborator

Fixes #481

Some contracts included in our deployment script are expected to use the Arachnid CREATE deployment proxy instead of the Safe singleton factory. We change our deployment scripts to manually use the Arachnid CREATE2 deployer in those specific cases, so the contracts end up at the expected addresses.

Affected contracts:

  • ERC-4337 entry point
  • Daimo P-256 verifier

You can verify this works by running pnpm run deploy hardhat in the Passkey package and checking the deployment addresses are what you expect them to be:

...
deploying "EntryPoint" (tx: 0x58b5726a3381344205508251ec394b4bf74b6f8bc669be0b1c9de8d75ce1f8ef)...: deployed at 0x0000000071727De22E5E9d8BAf0edAc6f37da032 with 3650209 gas
...
deploying "DaimoP256Verifier" (tx: 0xe18ff421fb23f117cdb2e60917f68703459743d0a26a14b79eae2cc01def4a20)...: deployed at 0xc2b78104907F722DABAc4C69f826a522B2754De4 with 814337 gas
...
deploying "SafeWebAuthnSignerFactory" (tx: 0x3e6df468f44301ff0bd64fd2403a42214417e25eb865a3c8cb68c79cc5ce042f)...: deployed at 0x1d31F259eE307358a26dFb23EB365939E8641195 with 1031063 gas
deploying "SafeWebAuthnSharedSigner" (tx: 0xc9d5201f64e0f0d537870bafb8a6445c70970f3a13fdda763eee0cb0519bfbf9)...: deployed at 0x94a4F6affBd8975951142c3999aEAB7ecee555c2 with 690536 gas

@nlordell
Use Arachnid CREATE2 Deployer For Some Contracts
5ae188e 
Fixes #481

Some contracts included in our deployment script are expected to use the
Arachnid CREATE deployment proxy instead of the Safe singleton factory.
We change our deployment scripts to manually use the Arachnid
CREATE2 deployer in those specific cases, so the contracts end up at the
expected addresses.

Affected contracts:
- ERC-4337 entry point
- Daimo P-256 verifier
@nlordell nlordell requested a review from a team as a code owner August 14, 2024 10:10
@nlordell nlordell requested review from akshay-ap, mmv08 and remedcu and removed request for a team August 14, 2024 10:10
@coveralls
Copy link

Pull Request Test Coverage Report for Build 10385621801

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%
Totals Coverage Status
Change from base Build 10372241868: 0.0%
Covered Lines: 92
Relevant Lines: 92
💛 - Coveralls

remedcu
remedcu reviewed Aug 15, 2024
View reviewed changes
Copy link
Member

@remedcu remedcu left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DaimoP256Verifier address is now using the Arachnid one, but the Entrypoint is still using the safe-singleton-factory when I checked with pnpm run deploy hardhat.

It was using the old files, rectified after doing build using the pnpm install in the root directory.

@remedcu remedcu self-requested a review August 15, 2024 12:37
remedcu
remedcu approved these changes Aug 15, 2024
View reviewed changes
Copy link
Member

@remedcu remedcu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated: benchmark check is failing, but it is unrelated to this PR.

@nlordell
Copy link
Collaborator Author

Unrelated: benchmark check is failing, but it is unrelated to this PR.

Should be fixed in #484

@remedcu remedcu merged commit df55dab into main Aug 15, 2024
10 of 11 checks passed
@remedcu remedcu deleted the chore/use-arachnid-deployer-for-some-contracts branch August 15, 2024 12:57
@github-actions github-actions bot locked and limited conversation to collaborators Aug 15, 2024
@mmv08
Copy link
Member

mmv08 commented Aug 19, 2024

Will we be able to deploy the contract to networks that enforce EIP-155?

@nlordell
Copy link
Collaborator Author

This only affects:

  • The Daimo P-256 verifier contract (which we will not be able to deploy)
  • The EntryPoint contract (which we will also not be able to deploy)

For the Daimo P-256 verifier, we can easily have exceptions for networks that don't have the Arachnid CREATE2 deployer (so it will just end up on a "non-canonical" address). It is also not a "core" Safe contract, so I don't see this as a huge issue.

For the EntryPoint contract, it is a bit more complicated (as the Safe4337Module will also end up on a different address), and I assume that getting bundler compatibility will also be a challenge. I would wait to see how the 4337 ecosystem handles this particular case before deciding what we do.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@remedcu remedcu remedcu approved these changes

@akshay-ap akshay-ap Awaiting requested review from akshay-ap akshay-ap is a code owner automatically assigned from safe-global/safe-protocol

@mmv08 mmv08 Awaiting requested review from mmv08 mmv08 is a code owner automatically assigned from safe-global/safe-protocol

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Daimo deployment update
4 participants
@nlordell @coveralls @mmv08 @remedcu